Copter Fence Fixes #25875

andyp1per · 2024-01-02T19:51:54Z

Separated out from #25698

Implements Min Alt fence on copter
Adds support for auto-enabling fence floors on copter
Messages correctly about fence breach / enablement
Auto-disable floor fence on landing on copter
Always checks breaches regardless of manual recovery state - means the scripting API is up-to-date
Prevent manual recovery logic from engaging when no fence action has been set - allows better control from scripting
Implements FENCE_AUTOENABLE for copter
Implements FENCE_OPTIONS for copter

Here is an attempt at describing what this is supposed to do:

A fence is a 2D plane in 3D space separating "inside" the fence from "outside" the fence
A fence can be in one of four incremental states (so e.g. a fence can only be active if it is already configured):
- Unconfigured - the fence is unavailable for use in any way
- Configured - the fence is able to be activated by some activation mechanism
- Active - the fence is active and will be put into the breached state if the vehicle is "outside" the fence
- Breached - the fence is active, the vehicle is "outside" the fence and any breach action will be taken
The state of each fence is fully independent of all of the others
A fence is configured by setting the appropriate bit in FENCE_TYPES
Fences come in two categories - Normal and Auto. By default Max Alt, Circle and Poly fences are considered Normal fences and Min Alt is considered an Auto fence.
A fence is activated by one of the following mechanisms:
- Setting FENCE_ENABLE to 1 activates all configured normal fences
- Setting FENCE_ENABLE to 1 activates all configured auto fences when their autoconditions become true
- Setting an RC switch, configured with option 11 (FENCE), high activates all configured fences
- Sending the mavlink message MAV_CMD_DO_FENCE_ENABLE actives all fences specified in the message
- If FENCE_AUTOENABLE is set to some value causes all normal fences to become auto fences that are activated when their auto conditions become true
A fence is de-activated by one of the following mechanisms:
- Setting FENCE_ENABLE to 0 de-activates all configured fences
- Setting an RC switch, configured with option 11 (FENCE), low de-activates all configured fences
- Sending the mavlink message MAV_CMD_DO_FENCE_ENABLE de-actives all fences specified in the message
- An automatic landing (RTL in Copter and in Plane if certain options are set, LAND, land in AUTO) will disable all auto fences and ignore all breach actions
The following auto-conditions are available:
- Ascending above Min Alt by default
- On arming via FENCE_AUTOENABLE
- On auto takeoff via FENCE_AUTOENABLE
The execution of a breach action prevents the execution of any further breach action until the breach action is completed or pilot changes mode or disarms

Requires ArduPilot/mavlink#349

ArduCopter/fence.cpp

tridge

I'd like @Hwurzburg to have a look at the impact on plane

libraries/AC_Fence/AC_Fence.cpp

Hwurzburg

I think a few changes are needed: disabling floor on disarm for all autoenable values if the option is set...the impact of long continued breaching flight in plane must be either accepted by @tridge or changed.
This does fix some Plane bugs #1 and #2 and #4 if the change to when disable_floor is done:

1.AutoEnable on arming, or not using AutoEnabled, never disables a floor for landing. So all the Plane situations which think they disable floor, wont.
2.MIN_ALT floor always disabled on boot irrespective FENCE_ENABLE, never gets enabled unless: autoenabled, or overt enable(GCS or switch).
3.MIN_ALT fence records breach even if floor is not enabled. MIN_ALT breach is recorded but not cleared after landing, preventing mode changes if mode change prevention is enabled (Copter allows mode changes always). This prevents mode changes after landing.
4.MIN_ALT floor stays disabled after disable floor for landing, used by NAV_LAND, QLAND and other VTOL landings in Plane, LAND, Copter RTL
5.Plane automatically ignores ANY breach while performing a breach action. Again prevents mode changes in Plane while landed.

libraries/AC_Fence/AC_Fence.cpp

andyp1per · 2024-01-04T20:54:06Z

Just to try and capture what @rmackay9 would like - he would prefer that we auto-disable the floor fence as we go through it on a land/RTL action rather than leaving the fence enabled and ignoring it

libraries/AC_Fence/AC_Fence.cpp

andyp1per · 2024-01-12T18:24:58Z

Trying to write down some rules here, as this stuff is convoluted ...

Fence presence (FENCE_TYPE) means a fence is eligible to be enabled, but nothing more
Fence enablement (FENCE_ENABLE) means a fence is active and will take action if breached. We currently have no way of selectively enabling but I am beginning to think we should.
Fence breach (get_breaches()) means a present and enabled fence has been breached. Breaches should always be correct, if you want to suspend a fence (e.g. to land) then it should be disabled.
Fence auto-enablement (FENCE_AUTOENABLE) allows present fences to be automatically enabled and disabled under certain conditions - on arming, on takeoff, on landing. It's not clear why auto-enablement of anything other than floor fences makes sense. Auto-enablement also suffers from memory loss - if a fence is auto-enabled and then directly enabled via mavlink (as we do in the tests), what happens when you land - should the (non-floor) fence be disabled or enabled?
Certain modes (RTL descent, Land, Auto) will automatically disable fences for landing so that they can land with a floor fence. It's not clear why this should be anything other than the floor fence.
Fences can be enabled and disabled via mavlink. Initially this was all fences, but more recent changes allow the floor fence to be switched off only. I am planning to add an option that does the opposite - allows all fences to be enabled except the floor.

The current behaviour is confusing and inconsistent because sometimes all fences are automatically disabled to take an action when only the floor fence needs to be disabled, similarly current behaviour ignores breached floor fences in certain (landing) circumstances. In order to get consistency I think we need individual control over presence (OK), enablement (MISSING) and breaches (OK). This means turning the enabled flag into a bitmask.

Hwurzburg · 2024-01-12T21:44:16Z

"Fence enablement (FENCE_ENABLE) means a fence is active and will take action if breached. We currently have no way of selectively enabling but I am beginning to think we should." is untrue
....if _floor_enable is false the ALT_MIN fence is disabled...currently starts in this state after boot even if FENCE_ENABLE is true at boot...takes a direct action to enable it or AUTOENABLE condition get floor enabled...

Personally, I think:

We should have the fence enable/disable AUX function have a bit mask variable for what it controls. The same for MAVLink fence command.
We should have auto enable and disable functions of ALT_MIN that can be called from takeoff and landing code. I am not sure why you would ever NOT want to disable the ALT_MIN fence on an auto landing...its a pilot control protection I think
We should have a fence option to disable horizontal fences during breach actions (RTLs) Plane just ignores breaches during breach recovery actions, but doesn't clear the breach...we should prevent a breach by allowing a temporary disable with auto-re-enable if breach action abort or when changing mode to re-takeoff
I can see the need for selectively auto-enabling non ALT_MIN fences on arming (to move out of an exclusion zone like pit area without breaching), but why its needed otherwise I am not sure. If we just enable the ALT_MIN once its reached, disable on auto landings, and re-enable once another climb above it occurs. Maybe we can deprecate the FENCE_AUTOENABLE and replace with a FENCE_OPTION for autoenabling non-ALT_MIN fences on arming. Not sure why you need the enable on takoff if you have the enable on arming. And making ALT_MIN autoenable removes the need for the normal enable on takoff.

andyp1per · 2024-01-13T15:43:31Z

"Fence enablement (FENCE_ENABLE) means a fence is active and will take action if breached. We currently have no way of selectively enabling but I am beginning to think we should." is untrue ....if _floor_enable is false the ALT_MIN fence is disabled...currently starts in this state after boot even if FENCE_ENABLE is true at boot...takes a direct action to enable it or AUTOENABLE condition get floor enabled...

_floor_enabled is not a parameter, so there is some indirect stuff going on, but nothing that allows you to directly enable or disable individual fences

Personally, I think:

* We should have the  fence enable/disable AUX function have a bit mask variable for what it controls. The same for MAVLink fence command.

Yes, 100% agree

* We should have auto enable and disable functions of ALT_MIN that can be called from takeoff and landing code. I am not sure why you would ever NOT want to disable the ALT_MIN fence on an auto landing...its a pilot control protection I think

Yes agree, auto enable of anything else seems a bit suspect to me

* We should have a fence option to disable horizontal fences during breach actions (RTLs) Plane just ignores breaches during breach recovery actions, but doesn't clear the breach...we should prevent a breach by allowing a temporary disable with auto-re-enable if breach action abort  or when changing mode to re-takeoff

Yes, so this is not possible without finer grained control of enablement - bu I agree this is a good way forward

* I can see the need for selectively auto-enabling non ALT_MIN fences on arming (to move out of an exclusion zone like pit area without breaching), but why its needed otherwise I am not sure. If we just enable the ALT_MIN once its reached, disable on auto landings, and re-enable once another climb above it occurs. Maybe we can deprecate the FENCE_AUTOENABLE and replace with a FENCE_OPTION for autoenabling non-ALT_MIN fences on arming.  Not sure why you need the enable on takoff if you have the enable on arming. And making ALT_MIN autoenable removes the need for the normal enable on takoff.

Agree about simplifying, still not quite sure what the correct mix is

andyp1per · 2024-01-13T19:37:46Z

I have turned FENCE_ENABLE into a bitmask and it makes everything much easier.

Hwurzburg · 2024-01-15T18:17:16Z

I think this is good, not recording an ALTMIN breach if not enabled....and having a configured ALT_MIN fence always autoenable (if not enabled by FENCE_ENABLE but configured in FENCE_TYPE) upon reaching the alt, and reenabling after a landing module disables it for landing if below the alt and a mode change occurs.....but we should also deprecate AutoEnable and convert "3" to setting a new FENCE_OPTIONS bit to enable all non MIN_ALT configured fences on arming, which would not be the default for the option bit
This will fix many problems in Plane and still function as desired (but better really)

libraries/AC_Fence/AC_Fence.cpp

Hwurzburg · 2024-07-20T22:30:41Z

here is another nasty one....Tridges patch fixes AUTOENABLE =2 operation, but AUTOENABLE =1 is broken still....first flight it will enable the poly on takeoff and breach and take breach action on the poly...but if you refly by restarting the mission, it will say that the poly fence has been enabled after takeoff, but not breach when it hits the poly....log :https://www.dropbox.com/scl/fi/1hdmsykaefv8yrhir92t9/00000003.BIN?rlkey=ajbgixqzzubbrby0wy5omvo9c&dl=0

tridge · 2024-07-20T22:51:03Z

@andyp1per @Hwurzburg this fixes the lastest issue that Henry found:
andyp1per#33

tridge · 2024-07-20T23:23:06Z

I think we should mark FENCE_AUTOENABLE=1 and FENCE_AUTOENABLE=2 as deprecated, not actually remove them yet, but issue a deprecation warning, and recommend not using them in the wiki, instead pointing users at either FENCE_ENABLE=1 or FENCE_AUTOENABLE=3

the is an adjustment to the previous fix which only worked when we had at least one fence element enabled when we were not flying or disarmed

libraries/AC_Fence/AC_Fence.cpp

andyp1per · 2024-07-21T15:05:12Z

I think we should mark FENCE_AUTOENABLE=1 and FENCE_AUTOENABLE=2 as deprecated, not actually remove them yet, but issue a deprecation warning, and recommend not using them in the wiki, instead pointing users at either FENCE_ENABLE=1 or FENCE_AUTOENABLE=3

I think these are all up for grabs, but would like to have that conversation after the PR goes in!

Hwurzburg · 2024-07-21T16:46:09Z

another issue...if you have setup a fence sw, you cant autoenable any more and that includes min_alt....if you set it high on the ground you breach min_alt...if its low, then autoenable (at least 3) does not enable any fences....I would think at least on boot, the fence switch would allow autoenable if low....ie like fence_enable = 0.
But i guess we could say that autoenable does not work if you have setup a fence sw.....I dont like it myself....I think that maybe we should change the fence sw init to do nothing and need a switch transition, or make the MIN_ALT work the same as in Autoenable...ie have to be above before enabling....anyway its very awkward now....

timtuxworth · 2024-07-21T17:13:40Z

another issue...if you have setup a fence sw, you cant autoenable any more and that includes min_alt....if you set it high on the ground you breach min_alt...if its low, then autoenable (at least 3) does not enable any fences....I would think at least on boot, the fence switch would allow autoenable if low....ie like fence_enable = 0. But i guess we could say that autoenable does not work if you have setup a fence sw.....I dont like it myself....I think that maybe we should change the fence sw init to do nothing and need a switch transition, or make the MIN_ALT work the same as in Autoenable...ie have to be above before enabling....anyway its very awkward now....

Yes - the switch should be "turn on fence function/turn off fence function" - when on, it should mean that fence functionality is on, (not that all fences turn on right away) - so autoenable will function if the switch is on, when off all fences are off and autoenable will not function.

andyp1per · 2024-07-21T17:44:42Z

I was trying to be very conservative. It’s very easy for mavlink not to work. Also switch behaviour needs to be idempotent which makes it largely incompatible with auto enable.Andy Sent from my iPhoneOn 21 Jul 2024, at 18:46, Henry Wurzburg ***@***.***> wrote: another issue...if you have setup a fence sw, you cant autoenable any more and that includes min_alt....if you set it high on the ground you breach min_alt...if its low, then autoenable (at least 3) does not enable any fences....I would think at least on boot, the fence switch would allow autoenable if low....ie like fence_enable = 0. But i guess we could say that autoenable does not work if you have setup a fence sw.....I dont like it myself....I think that maybe we should change the fence sw init to do nothing and need a switch transition, or make the MIN_ALT work the same as in Autoenable...ie have to be above before enabling....anyway its very awkward now.... —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

timtuxworth · 2024-07-21T17:59:08Z

Also switch behaviour needs to be idempotent which makes it largely incompatible with auto enable.

Does it make sense If you think of the switch as "enable/disable Fence capability" rather than "enable/disable the actual fence"?

andyp1per · 2024-07-21T21:14:55Z

Also switch behaviour needs to be idempotent which makes it largely incompatible with auto enable.

Does it make sense If you think of the switch as "enable/disable Fence capability" rather than "enable/disable the actual fence"?

I can't change the current meaning of what happens on a switch, the behaviour should be the same

Hwurzburg · 2024-07-22T16:47:15Z

With a couple of exceptions, ALT_MIN fence, if configured, will become enabled once MIN_ALT is obtained. It is disabled on NAV_LAND. Adding a margin seems like a good idea...maybe +2m
-AUTOENABLE =1 enables it at takeoff alt completion (I think this should change for consistency)
-AUX switch high for fence instantly enables all fences including MIN_ALT (I think this should change for consistency)
FENCE_ENABLE=1 enables all fences except MIN_ALT until its reaches MIN_ALT. Dynamically changing FENCE_ENABLE to 0 disables all fences immediately. FENCE_AUTOENABLEs need FENCE_ENABLE= 0 to work.
Fence AUX switch change enable or disables, omnipotently. Sw value is scanned at boot (init'd). This results in MIN_ALT breach on ground preventing boot. I think this should change, as stated above in 1.
I agree with deprecating AUTOENABLE to only be off or on ARM only.

Stale

tridge · 2024-07-23T00:04:02Z

@andyp1per this PR makes the change to your PR as requested by @IamPete1
andyp1per#34
if we merge that then I believe we will then be able to merge this PR

when the user has chosen to disallow mode change during fence breach it should be fully implemented, without a landing exception. as requested by Pete, and discussed on dev call

andyp1per · 2024-07-23T08:17:16Z

@andyp1per this PR makes the change to your PR as requested by @IamPete1 andyp1per#34 if we merge that then I believe we will then be able to merge this PR

Merged thanks @tridge

andyp1per force-pushed the pr-copter-fences branch from fb53b8b to 67fdd74 Compare January 2, 2024 20:05

andyp1per mentioned this pull request Jan 2, 2024

Fast Copter Fences #25698

Merged

andyp1per force-pushed the pr-copter-fences branch from 67fdd74 to e1534f5 Compare January 2, 2024 20:07

andyp1per added BUG DevCallEU labels Jan 2, 2024

Hwurzburg reviewed Jan 2, 2024

View reviewed changes

ArduCopter/fence.cpp Outdated Show resolved Hide resolved

andyp1per force-pushed the pr-copter-fences branch from e1534f5 to 451f738 Compare January 2, 2024 22:05

tridge requested changes Jan 3, 2024

View reviewed changes

libraries/AC_Fence/AC_Fence.cpp Outdated Show resolved Hide resolved

tridge removed the DevCallEU label Jan 3, 2024

Hwurzburg requested changes Jan 3, 2024

View reviewed changes

libraries/AC_Fence/AC_Fence.cpp Outdated Show resolved Hide resolved

libraries/AC_Fence/AC_Fence.cpp Outdated Show resolved Hide resolved

libraries/AC_Fence/AC_Fence.cpp Outdated Show resolved Hide resolved

libraries/AC_Fence/AC_Fence.cpp Show resolved Hide resolved

Hwurzburg reviewed Jan 4, 2024

View reviewed changes

libraries/AC_Fence/AC_Fence.cpp Outdated Show resolved Hide resolved

libraries/AC_Fence/AC_Fence.cpp Outdated Show resolved Hide resolved

andyp1per force-pushed the pr-copter-fences branch from 3fee4bd to 014fa54 Compare January 13, 2024 18:10

andyp1per requested review from Hwurzburg and tridge January 13, 2024 19:36

andyp1per force-pushed the pr-copter-fences branch 2 times, most recently from 64e56a7 to 8034156 Compare January 14, 2024 14:12

Hwurzburg added the DevCallTopic label Jan 15, 2024

tridge requested changes Jan 15, 2024

View reviewed changes

libraries/AC_Fence/AC_Fence.cpp Outdated Show resolved Hide resolved

tridge added DevCallEU and removed DevCallTopic DevCallEU labels Jan 15, 2024

andyp1per force-pushed the pr-copter-fences branch 3 times, most recently from 3963290 to 34b5f35 Compare January 19, 2024 20:49

Plane: fixed re-enable of fence for FENCE_AUTOENABLE=1

398fe33

the is an adjustment to the previous fix which only worked when we had at least one fence element enabled when we were not flying or disarmed

Hwurzburg requested changes Jul 21, 2024

View reviewed changes

libraries/AC_Fence/AC_Fence.cpp Outdated Show resolved Hide resolved

andyp1per force-pushed the pr-copter-fences branch from 7656c17 to 961e47e Compare July 21, 2024 14:55

andyp1per added 4 commits July 21, 2024 17:02

AP_Mission: address minor review comments

89e7220

AC_Fence: address minor review comments

768f780

Copter: address minor review comments

96d9985

Plane: address minor review comments

8d7db72

andyp1per force-pushed the pr-copter-fences branch from 5753aa6 to 8d7db72 Compare July 21, 2024 15:02

andyp1per requested review from IamPete1 and Hwurzburg July 21, 2024 15:24

CraigElder removed the DevCallTopic label Jul 22, 2024

tridge approved these changes Jul 22, 2024

View reviewed changes

Hwurzburg approved these changes Jul 22, 2024

View reviewed changes

Copter: fully honour FENCE_OPTION to disable mode changes

166cb48

when the user has chosen to disallow mode change during fence breach it should be fully implemented, without a landing exception. as requested by Pete, and discussed on dev call

peterbarker merged commit a371a3e into ArduPilot:master Jul 23, 2024
93 checks passed

andyp1per deleted the pr-copter-fences branch July 24, 2024 06:02

IamPete1 mentioned this pull request Jul 24, 2024

Plane: move have_autoenabled_fences to be private in ModeTakeoff #27638

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Copter Fence Fixes #25875

Copter Fence Fixes #25875

andyp1per commented Jan 2, 2024 •

edited

Loading

tridge left a comment

Hwurzburg left a comment •

edited

Loading

andyp1per commented Jan 4, 2024

andyp1per commented Jan 12, 2024

Hwurzburg commented Jan 12, 2024 •

edited

Loading

andyp1per commented Jan 13, 2024

andyp1per commented Jan 13, 2024

Hwurzburg commented Jan 15, 2024 •

edited

Loading

Hwurzburg commented Jul 20, 2024

tridge commented Jul 20, 2024

tridge commented Jul 20, 2024

andyp1per commented Jul 21, 2024

Hwurzburg commented Jul 21, 2024

timtuxworth commented Jul 21, 2024

andyp1per commented Jul 21, 2024 via email

timtuxworth commented Jul 21, 2024

andyp1per commented Jul 21, 2024

Hwurzburg commented Jul 22, 2024 •

edited

Loading

tridge commented Jul 23, 2024

andyp1per commented Jul 23, 2024

Copter Fence Fixes #25875

Copter Fence Fixes #25875

Conversation

andyp1per commented Jan 2, 2024 • edited Loading

tridge left a comment

Choose a reason for hiding this comment

Hwurzburg left a comment • edited Loading

Choose a reason for hiding this comment

andyp1per commented Jan 4, 2024

andyp1per commented Jan 12, 2024

Hwurzburg commented Jan 12, 2024 • edited Loading

andyp1per commented Jan 13, 2024

andyp1per commented Jan 13, 2024

Hwurzburg commented Jan 15, 2024 • edited Loading

Hwurzburg commented Jul 20, 2024

tridge commented Jul 20, 2024

tridge commented Jul 20, 2024

andyp1per commented Jul 21, 2024

Hwurzburg commented Jul 21, 2024

timtuxworth commented Jul 21, 2024

andyp1per commented Jul 21, 2024 via email

timtuxworth commented Jul 21, 2024

andyp1per commented Jul 21, 2024

Hwurzburg commented Jul 22, 2024 • edited Loading

tridge commented Jul 23, 2024

andyp1per commented Jul 23, 2024

andyp1per commented Jan 2, 2024 •

edited

Loading

Hwurzburg left a comment •

edited

Loading

Hwurzburg commented Jan 12, 2024 •

edited

Loading

Hwurzburg commented Jan 15, 2024 •

edited

Loading

Hwurzburg commented Jul 22, 2024 •

edited

Loading